Skip to content

Version 2 - #9

Merged
devgioele merged 26 commits into
mainfrom
major-refactor
Jul 20, 2023
Merged

Version 2#9
devgioele merged 26 commits into
mainfrom
major-refactor

Conversation

@devgioele

@devgioele devgioele commented Jul 19, 2023

Copy link
Copy Markdown

There are now 3 hooks: useQuery, usePagination and useQueryAndPagination.
Those exist for each router: Next.JS router, React Router and in-memory router.
In addition, a more convenient version for zod is exported.

Have a look at the package.json listing all the entry points. The entry points are specified under the exports key. The keys main, types and module are not necessary since Node.JS 12+.

The old tests have been recycled as much as possible.

Changes in behavior:

  • Changing the remaining query does no longer reset pagination.
  • The query is parsed with the given parser. The parser can throw an exception if the parsing is not possible.

Have a look at the new readme.md for an introduction to version 2.

@devgioele devgioele self-assigned this Jul 19, 2023
@devgioele

Copy link
Copy Markdown
Author

I am still trying to fix the building and exportation so that one does not always need next and react-router-drom to be installed.

@devgioele
devgioele marked this pull request as draft July 19, 2023 08:10
@devgioele
devgioele marked this pull request as ready for review July 19, 2023 08:36
@devgioele

Copy link
Copy Markdown
Author

The exportation has been solved! Works great with multiple entry points!

Comment thread readme.md Outdated
```

Second, you can make use of the `useQueryAndPagination` hook. This package implements 3 versions of this hook:
There are a variety of entry points from which to import the hooks `useQuery`, `usePagiation` and `useQueryAndPagination`:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
There are a variety of entry points from which to import the hooks `useQuery`, `usePagiation` and `useQueryAndPagination`:
There are a variety of entry points from which to import the hooks `useQuery`, `usePagination` and `useQueryAndPagination`:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has been fixed, thank you

Comment thread readme.md Outdated
```

## Information
## About

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep the old title. We have that one in every package.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright

Comment thread src/zod/routers/nextRouter.ts Outdated
>(
defaultQuery: TQuery,
schemaQuery: ZodType<TSchemaOutput, TSchemaDef, TSchemaInput>,
defaultPagination?: PaginationQuery

@mmalfertheiner mmalfertheiner Jul 20, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the NextRouterOptions are missing here and in the useQuery hook.

options?: Partial<AbstractQueryOptions & NextRouterOptions>

Please also check the other zod routers.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! The options have been added

Comment thread src/engine/pagination.ts Outdated
Comment on lines +11 to +13
export type PaginationQuery = { page: number; size: number }

export type ChangeQueryOptions = { resetPage: boolean }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add comments to describe the values. I think especially for resetPage it would not hurt and might not be immediately clear from the name. It would also be nice to see the default value for page, size and resetPage.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have a look at what I added. I think it is explained now.

Comment thread src/engine/pagination.ts

export type PaginationQuery = {
/**
* The page index.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the default value here and for the size as well?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mmalfertheiner
mmalfertheiner self-requested a review July 20, 2023 08:07

@lukasvice lukasvice left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! LGTM!

@devgioele
devgioele changed the base branch from improve-eslint to main July 20, 2023 16:44
@devgioele
devgioele merged commit 35df194 into main Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants